Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tech: plus d'erreur lorsque l'email de transfert n'a plus de dossier associé #9935

Merged
merged 2 commits into from
Jan 29, 2024

Conversation

colinux
Copy link
Member

@colinux colinux commented Jan 24, 2024

https://demarches-simplifiees.sentry.io/issues/4880772787

Quand un usager transfer un dossier, et que dans la foulée il le supprime avant que le mail soit parti, l'envoie échoue.

  • prévient l'usager que la suppression entraine aussi la suppression du transfert
  • fait en sorte que le mailer ne passe plus par la view et n'envoie pas le mail 🥵

@colinux colinux marked this pull request as ready for review January 25, 2024 09:28
@mfo mfo force-pushed the transfer-empty-no-fail branch from 954a8da to 9e09e0e Compare January 25, 2024 09:28
@colinux colinux force-pushed the transfer-empty-no-fail branch from 9e09e0e to 50e45ac Compare January 25, 2024 09:56
@colinux colinux requested a review from mfo January 25, 2024 09:56
@colinux colinux force-pushed the transfer-empty-no-fail branch from 50e45ac to ed4d4e2 Compare January 25, 2024 10:04
…ween queuing and processing

Co-authored-by: mfo <mfo@users.noreply.github.com>
@colinux colinux force-pushed the transfer-empty-no-fail branch from ed4d4e2 to 4aa1306 Compare January 25, 2024 10:31
@colinux colinux enabled auto-merge January 25, 2024 11:38
@colinux colinux added the bug label Jan 26, 2024
@colinux colinux added this pull request to the merge queue Jan 29, 2024
Merged via the queue into demarches-simplifiees:main with commit b64a324 Jan 29, 2024
15 checks passed
@colinux colinux deleted the transfer-empty-no-fail branch January 29, 2024 08:55
@@ -503,6 +503,8 @@ fr:
start_other_dossier: "Commencer un autre dossier vide"
clone: "Dupliquer ce dossier"
delete_dossier: "Supprimer le dossier"
delete_dossier_confirm: "En continuant, vous allez supprimer ce dossier ainsi que les informations qu’il contient. Toute suppression entraîne l’annulation de la démarche en cours.\n\nConfirmer la suppression ?"
delete_dossier_with_transfer_confirm: "En continuant, vous allez supprimer ce dossier, les informations qu’il contient ainsi que sa demande de transfert. Toute suppression entraîne l’annulation de la démarche en cours.\n\nConfirmer la suppression ?"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Peut être rappeler vers qui est destiné le transfert. L'idée étant que voir le mail du destinataire rend plus compréhensible le transfert.

Suggested change
delete_dossier_with_transfer_confirm: "En continuant, vous allez supprimer ce dossier, les informations qu’il contient ainsi que sa demande de transfert. Toute suppression entraîne l’annulation de la démarche en cours.\n\nConfirmer la suppression ?"
delete_dossier_with_transfer_confirm: "En continuant, vous allez supprimer ce dossier, les informations qu’il contient ainsi que sa demande de transfert à %{email}. Toute suppression entraîne l’annulation de la démarche en cours.\n\nConfirmer la suppression ?"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bien vu je le rajouterai dans une autre PR vu que c'est déjà mergé

def notify_transfer(transfer)
I18n.with_locale(transfer.user_locale) do
def notify_transfer
@transfer = params[:dossier_transfer]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pour ma culture, pourquoi préférer cette écriture à notify_transfert(transfer) ?

Copy link
Member Author

@colinux colinux Jan 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

car sinon la méthode est exécutée immédiatement et on ne peut plus empêcher de passer par la vue. En quelque sorte les before/after action ont moins de "pouvoir" si on fait directement Mailer.action. C'est ce qui nous a pris pas mal de temps à comprendre .
bigup à @mfo d'avoir débuggué et compris cet aspect là

Copy link

sentry-io bot commented Jan 29, 2024

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ **NoMethodError: undefined method []' for nil (NoMethodError)** PriorizedMailDeliveryJob` View Issue

Did you find this useful? React with a 👍 or 👎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants